Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give a kind to all node[] fields and better types in Java-generated code #2497

Merged
merged 8 commits into from
Feb 28, 2024

Conversation

eregon
Copy link
Member

@eregon eregon commented Feb 25, 2024

@eregon
Copy link
Member Author

eregon commented Feb 25, 2024

I was wondering if CI would catch that InterpolatedStringNode#parts could be not only:

        kind:
          - StringNode
          - EmbeddedStatementsNode

But also EmbeddedVariableNode (which I figured out by checking prism.c).
But unfortunately the CI does not catch this.
I guess we would need some kind of runtime type checks for this, and steep seems to only do static type checks.

@eregon
Copy link
Member Author

eregon commented Feb 25, 2024

Is there maybe a way to do runtime type checks using Sorbet?
I read https://sorbet.org/docs/runtime quickly but did not find an easy way to enable runtime checks using the .rbi files we have.
Maybe @paracycle or someone else would know better about that?

@eregon eregon marked this pull request as ready for review February 25, 2024 13:29
@eregon
Copy link
Member Author

eregon commented Feb 25, 2024

I added my own runtime checks in nodes initialize constructors, only added by the template if $CHECK_FIELD_KIND is set.

Copy link
Collaborator

@kddnewton kddnewton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! One small suggestion that I think will make this is a little easier to test. Also please rebase off #2517 after it's merged.

config.yml Outdated Show resolved Hide resolved
config.yml Outdated Show resolved Hide resolved
config.yml Outdated Show resolved Hide resolved
config.yml Outdated Show resolved Hide resolved
@kddnewton kddnewton merged commit aa42f45 into ruby:main Feb 28, 2024
54 checks passed
@eregon eregon deleted the better-java-types branch February 28, 2024 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding kind for node[] fields where possible
2 participants